Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zephyr: decouple from platform pm_runtime.h interface #9495

Merged
merged 5 commits into from
Sep 24, 2024

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Sep 19, 2024

This is somewhat complicated series to remove the platform pm_runtime.h layer from SOF builds and greatly simplify the remaining code used in Zephyr builds.

Key assumptions:

  • only Intel platforms have had non-trivial pm_runtime implementations
  • all Intel platforms are Zephyr-only (not possible to build for XTOS anymore)
  • the remaining pm_runtime usage in SOF application code can be turned into generic Zephyr calls
  • => we can completely drop the platform pm_runtime.h layer from Zephyr builds

The initialization of DSP state is now done in Zephyr,
so the calls to initialize DSP core and memory state are
no longer needed and can be removed.

Signed-off-by: Kai Vehmanen <[email protected]>
Since commit 35b17a4 ("platform: tigerlake: remove XTOS platform
definitions"), the platform_pm_runtime_get() has been a no-op call
with a dummy implementation in zephyr/lib/pm_runtime.c.

Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Sep 19, 2024

@LaurentiuM1234 and @thesofproject/nxp , please review. I added sof/zephyr/lib/pm_runtime.c to all Zephyr builds, including NXP ones. This is needed to handle the few references on pm_runtime call in SOF app code. I did not yet remove the platform pm_runtime.h files for NXP targets, but you could now do that as well.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Sep 19, 2024

@tmleman please check, should not impact Intel targets in practise (functionality not changed), but given amount of code moved around, better to check.

@@ -461,6 +458,7 @@ zephyr_library_sources(
schedule.c
lib/alloc.c
lib/cpu.c
lib/pm_runtime.c
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

am I missing something? The commit description says, that this drops pm_runtime.c from Zephyr builds?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh maybe I need to reword, but the patch drops the common sof/src/lib/pm_runtime.c and instead links sof/zephyr/lib/pm_runtime.c.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i oh, ok, thanks, that makes it clear even to me :-) If you end up updating, maybe yes, a bit of clarification could help

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh git commit clarified in updated series that I just pushed

/* prevent DSP Common power gating */
pm_runtime_get(PM_RUNTIME_DSP, PLATFORM_PRIMARY_CORE_ID);

#if CONFIG_DSP_RESIDENCY_COUNTERS
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this configuration option is now unused and can be removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh It's still implemented in XTOS pm_runtime.c and would be removing a feature from XTOS builds. So keeping in at least for now.

Copy link
Contributor

@LaurentiuM1234 LaurentiuM1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested on imx8 with an aplay.

If not too much of a hassle, mind also including #9503 in this series? It's the cleanup for imx platforms.

Copy link
Contributor

@tmleman tmleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

kv2019i and others added 3 commits September 23, 2024 18:25
SOF defines a pm_runtime.h interface that consists of a RTOS
layer and a platform specific layer.

So far, the interface has been used in driver code and a very
small set of audio modules (e.g. key phrase buffer in kpb.c).
The platform layer has only been implemented on Intel platforms.
A no-op implementation is used for other platforms.

As all Intel platforms have moved to Zephyr, this now allows to
simplify the code a lot for SOF Zephyr builds and drop all
dependencies to the pm_runtime.h platform layer. With Zephyr,
the drivers are defined on Zephyr side and the device runtime
management can be handled with Zephyr device interfaces.

This patch removes all use of the pm_runtime.h platform layer in Zephyr
builds and replaces linkage of generic sof/src/lib/pm_runtime.c with
sof/zephyr/lib/pm_runtime.c. The Zephyr sof/lib/pm_runtime.h provides
sufficient functionality to cover all uses of pm_runtime interface in
SOF application code (e.g. init and kbp).

The changes simplify the codebase and reduce the amount of
boilerplate code needed to add new hardware targets to SOF, when
Zephyr is used as the RTOS.

Signed-off-by: Kai Vehmanen <[email protected]>
Platform implementation no longer mandatory for SOF Zephyr builds,
so the pm_runtime.h file can be removed for these platforms.

Signed-off-by: Kai Vehmanen <[email protected]>
Remove pm_runtime.h file from some imx platforms
(imx93, imx8, and imx8ulp), which only support Zephyr
builds. This is done because the header is no longer
mandatory. The operations implemented in it were NOPs.

Signed-off-by: Laurentiu Mihalcea <[email protected]>
Signed-off-by: Kai Vehmanen <[email protected]>
@kv2019i
Copy link
Collaborator Author

kv2019i commented Sep 23, 2024

New version pushed:

@kv2019i kv2019i merged commit f2efd4a into thesofproject:main Sep 24, 2024
42 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants